Fix checkJs behavior for .mjs and .cjs files next to declarations - #64527
Hardik K (hardikkaurani) wants to merge 4 commits into
Conversation
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
Looks like this PR has a chance to resolve #63523 too. 😀 |
|
Jimmy Leung (@hkleungai) Great catch! Yes, this PR should indeed fix the inconsistency reported in #63523. By adding the identical exemption to Ryan Cavanaugh (@RyanCavanaugh) Just a heads up! I've linked this PR to #64312 to ensure consistent behavior for .mjs/.d.mts\ and .cjs/.d.cts\ files, mirroring the existing .js/.d.ts\ legacy behavior. As an added benefit, this also patches the wildcard parsing order dependency noted in #63523, making the behavior deterministic regardless of the \include\ array order. Let me know if you have any questions or feedback! |
|
No tests? |
|
Jake Bailey (@jakebailey) Thanks for the feedback! I've added a regression test in |
|
...Why would we make the new extensions have an explicitly legacy behavior? |
|
If anything, we should remove the legacy lookup behavior from |
|
Wesley Wigham (@weswigham) I looked further into the existing extension-priority behavior and its history before making any changes. The Given that, extending the existing For #64312, the issue explicitly prioritizes consistency over which behavior is chosen. One consistent approach would therefore be to remove the legacy Since that would intentionally change existing |
One can imagine that, if projects are running so fast, if old js source files keep growing, then adding explicit declaration files would be (one of) the most non-interuptive way for adding some flavour of typechecking. Kinda like C-style codebase, with all the I am intentionally skipping the inline jsdoc approach in above. In terms of typegen, for some people, jsdoc is a bit harder to be done right, than having explicit typedef files. Not sure whether or not this coexisting pattern is becoming too uncommon in js world, but I would say in older versions of typescript, it is a possible way to do things :) |
|
Jimmy Leung (@hkleungai) That's a fair point. Authoring a The distinction here, though, is between the validity of that pattern and how the compiler handles those files when they are discovered together through wildcard inclusion. When TypeScript discovers both If a project needs to type-check the JavaScript implementation against its declarations, it can explicitly include both files via the That still leaves the compatibility question around the existing |
Imo, "switching" to Or I should say, this approach of digging deep into the distinction on Naively, in user perspective, I kinda hope I can put the all these sources as-is on |
|
Jimmy Leung (@hkleungai) I agree with that. I don't think the intended fix should require users to switch from I only mentioned explicit inclusion to distinguish intentional coexistence from wildcard discovery; I don't mean to suggest For this PR, I think the important question is how I'll leave the compatibility/design decision around the existing |
|
Wesley Wigham (@weswigham) i intentionally include a .js and .d.ts in all of my typed npm packages, and intend to continue to do so. That's the only way to write typed JS (without writing TS) that I'm aware of. |
|
Jordan Harband (@ljharb) Thanks for sharing that context! It's really helpful to see concrete examples of this pattern being actively used in the ecosystem. Wesley Wigham (@weswigham) Given Jordan's feedback that authoring a If so, extending this existing behavior to Would you be open to proceeding with the current approach in light of this? |
No.
Yeah, that's normal, and has no bearing on If you mean "I write my packages with manually authored I'd be happier to fix
We emit declaration files for jsdoc annotated js, which, as of TS7, has a much much closer correspondence to how we check and declaration emit TS files. That is the well-supported, non-footgun "how do I check my JS" path - not the weird |
…r maintainer feedback
|
Wesley Wigham (@weswigham) I've updated the implementation according to your feedback. We removed the legacy .js exception from the extension priority logic during wildcard resolution instead of extending it to modern extensions. This aligns legacy .js behavior with modern .mjs/.cjs behavior, creating a consistent and deterministic include resolution that prevents the configuration footguns. I've also updated the regression tests to assert that .js, .mjs, and .cjs implementations are consistently skipped when higher priority .d.ts, .d.mts, and .d.cts files are present. |
Currently, when
allowJs: trueandcheckJs: trueare enabled,.jsfiles located next to.d.tsfiles are included for type checking due to a legacy exception in the extension priority rules. However,.mjsand.cjsfiles alongside their.d.mtsand.d.ctscounterparts were skipped.This PR aligns the behavior of
.mjsand.cjsfiles with.jsfiles by extending the legacy exemption inhasFileWithHigherPriorityExtensionandremoveWildcardFilesWithLowerPriorityExtension. This ensures consistent behavior across all JS extensions, as requested by users.Fixes #64312